Skip to content

Fix: handle upload stream with unknown size#1481

Open
Hydrog3n wants to merge 1 commit into
minio:masterfrom
Hydrog3n:fix/handle-stream-with-unknown-size
Open

Fix: handle upload stream with unknown size#1481
Hydrog3n wants to merge 1 commit into
minio:masterfrom
Hydrog3n:fix/handle-stream-with-unknown-size

Conversation

@Hydrog3n

@Hydrog3n Hydrog3n commented Jun 16, 2026

Copy link
Copy Markdown

Hello, first thank you for the lib 🙏🏼

I arrived on a case where I have a stream from a DB who are converted to csv through pipe and uploading to S3.

But in certain case I have no data and I got this error since I update from 8.0.5 => 8.0.7

S3Error: The XML you provided was not well-formed or did not validate against our published schema

I'm open to improvement.

Summary by CodeRabbit

Bug Fixes

  • Improved multipart upload handling with refined part number sequencing and matching logic for better reliability during large file uploads.
  • Enhanced fallback mechanism for edge cases that may occur during the upload process.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e2b1ae8-d03c-4721-8ec9-3163691c9d75

📥 Commits

Reviewing files that changed from the base of the PR and between f871afd and 2b43dc7.

📒 Files selected for processing (1)
  • src/internal/client.ts

📝 Walkthrough

Walkthrough

In uploadStream inside src/internal/client.ts, the multipart chunk loop's partNumber is now initialized to 0 and incremented before the oldParts lookup, changing the part-number-to-chunk mapping. A new post-loop guard aborts the multipart upload and falls back to uploadBuffer with an empty object when eTags is empty.

Changes

Multipart Upload Part Numbering and Empty-Parts Fallback

Layer / File(s) Summary
Part number indexing and oldParts reuse logic
src/internal/client.ts
partNumber now starts at 0 and is incremented before the oldParts lookup, changing the mapping between chunk order and part numbers used when building eTags.
Empty eTags abort and fallback to uploadBuffer
src/internal/client.ts
After chunk processing, a new guard checks whether eTags is empty; if so, it aborts the multipart upload and calls uploadBuffer with an empty buffer instead of completing multipart with no parts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A part number shifted, now starts from zero,
The chunks line up right — this rabbit's a hero!
An empty eTags? Abort and retreat,
Upload a blank buffer, keep the flow neat.
No empty multipart shall pass on my watch,
The stream hops along without a botch! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: handle upload stream with unknown size' directly addresses the main objective of the PR, which is to fix handling of upload streams with unknown size. It is concise, specific, and clearly summarizes the primary change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@prakashsvmx

Copy link
Copy Markdown
Member

Please add unit/functional test @Hydrog3n

@Hydrog3n

Copy link
Copy Markdown
Author

@prakashsvmx Yep I was on it sorry 🙂

@prakashsvmx prakashsvmx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test with minio, aws s3 on both and present results

Comment thread src/internal/client.ts
for await (const chunk of chunkier) {
const md5 = crypto.createHash('md5').update(chunk).digest()

partNumber++

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be compatibility tested for partNumber

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants